src/services/destructive-command-guard/__tests__/runner.spec.ts (1)

122-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a case for signal-terminated exits.

The tests cover exit code 2 but never a non-null signal. runner.ts line 61 treats signal || (code !== 0 && code !== 1) as a failure, so a signal-terminated run with code 0 must still reject. Add a case that emits close with a signal to lock that branch.

🧪 Proposed test

+	it("rejects runs terminated by a signal", async () => {
+		const child = createChild()
+		useChild(child)
+
+		const result = runDcg("/dcg", "echo test", "/workspace")
+		child.stdout.write(JSON.stringify({ schema_version: 1, decision: "allow" }))
+		child.emit("close", 0, "SIGKILL")
+
+		await expect(result).rejects.toThrow("DCG evaluation failed")
+	})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/destructive-command-guard/__tests__/runner.spec.ts` around lines
122 - 132, Add a test alongside “rejects non-DCG exit statuses with stderr” that
emits the child’s close event with a non-null signal and exit code 0, then
assert the runDcg promise rejects. Reuse createChild, useChild, and the existing
result setup to cover the signal failure branch in the runner.

src/services/destructive-command-guard/__tests__/manager.spec.ts (3)

244-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the silent early return with skipIf.

Line 247 returns when the current platform is supported. Vitest then reports a passing test with zero assertions on every supported platform. The ZIP test at line 332 already uses it.skipIf. Use the same pattern here, and at line 263, so CI output states that the test was skipped.

♻️ Proposed change

-	it("warns when the current platform is unsupported", async () => {
-		const platformKey = `${process.platform}-${process.arch}`
-		const info = DCG_ARCHIVES[platformKey]
-		if (!info) return
+	const currentPlatformKey = `${process.platform}-${process.arch}`
+
+	it.skipIf(!DCG_ARCHIVES[currentPlatformKey])("warns when the current platform is unsupported", async () => {
+		const platformKey = currentPlatformKey
+		const info = DCG_ARCHIVES[platformKey]!

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/destructive-command-guard/__tests__/manager.spec.ts` around
lines 244 - 247, Replace the silent `if (!info) return` early return in the
platform warning test with Vitest’s `it.skipIf` pattern, and apply the same
change to the related test around `DCG_ARCHIVES` at line 263. Preserve the
existing test bodies while ensuring supported platforms are reported as skipped
rather than passing with zero assertions.

---

218-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Document the remaining double assertions.

Lines 168, 200, 288, 292, and 310 explain each as unknown as cast with a comment. Lines 218, 352, and 355 use the same casts without a comment. Add the same short explanation for consistency.

As per coding guidelines: "Use double assertions only as a last resort and explain them with a comment."

Also applies to: 348-356

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/destructive-command-guard/__tests__/manager.spec.ts` at line
218, Add concise explanatory comments beside the remaining double assertions in
the test setup around mockSpawn and the lines also identified near 348–356.
Match the existing comments used at lines 168, 200, 288, 292, and 310, without
changing the assertions or test behavior.

Source: Coding guidelines

---

302-308: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle rejections in the async setImmediate callbacks.

writeFile runs inside an async callback that no one awaits. If writeFile rejects, the rejection is unhandled, the test hangs on firstInstallation, and the real cause is hidden. The same pattern exists at lines 363-370. Catch the error and fail the child process instead.

♻️ Proposed change

 			setImmediate(async () => {
-				if (executable === "tar") {
-					const stagingDir = args[args.indexOf("-C") + 1]
-					await writeFile(path.join(stagingDir, info.binary), "executable")
-				}
-				child.emit("close", 0)
+				try {
+					if (executable === "tar") {
+						const stagingDir = args[args.indexOf("-C") + 1]
+						await writeFile(path.join(stagingDir, info.binary), "executable")
+					}
+					child.emit("close", 0)
+				} catch (error) {
+					child.emit("error", error)
+				}
 			})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/destructive-command-guard/__tests__/manager.spec.ts` around
lines 302 - 308, Handle promise rejections in both async setImmediate callbacks
around the child process simulation, including the callback near the tar
executable setup and the matching callback later in the test. Wrap writeFile and
subsequent logic in try/catch, and on failure emit a nonzero child close/error
result so the awaiting installation rejects instead of hanging.